Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve condition #2874

Closed
wants to merge 3 commits into from
Closed

Improve condition #2874

wants to merge 3 commits into from

Conversation

ngocnhan-tran1996
Copy link
Contributor

@ngocnhan-tran1996 ngocnhan-tran1996 commented Oct 19, 2024 •
edited
Loading

This PR includes:

  • Check this.producer before calling lock() to createOrGetProducer() and close method
  • Modernize switch pattern
  • Reduce else condition

@artembilan
Copy link
Member

@ngocnhan-tran1996 ,
Thank you for contribution!
Looks like you are addressing the other PR : #2862. Can yo mention that then and other contributor know, so we all don’t spend time looking into duplication?

@ngocnhan-tran1996 ngocnhan-tran1996 mentioned this pull request Oct 19, 2024
Closed
artembilan
artembilan approved these changes Oct 21, 2024
View reviewed changes
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pulling locally for final review and merge...

@artembilan artembilan added this to the 3.2.0-RC1 milestone Oct 21, 2024
@artembilan
Copy link
Member

Merged as 33da2a6.

@ngocnhan-tran1996 ,

thank you for contribution; looking forward for more!

A couple remarks:

Why your fork is called spring-amqp-fork? Why it cannot be just spring-amqp in your repositories?
Please, make yourself familiar with this article to understand of the commit message importance: https://cbea.ms/git-commit/.
Plus, GitHub is able to turn the first commit of the PR to its description.
This way we can save some time for extra typing.

@artembilan artembilan closed this Oct 21, 2024
@artembilan
Copy link
Member

This PR has also fixed #2861.

@ngocnhan-tran1996
Copy link
Contributor Author

Thanks @artembilan

I have a private repository name spring-amqp for some examples related spring-amqp. So I will rename it.

@ngocnhan-tran1996 ngocnhan-tran1996 deleted the improve-code branch October 23, 2024 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artembilan artembilan artembilan approved these changes

@acogoluegnes acogoluegnes acogoluegnes left review comments

@panchenko panchenko panchenko left review comments

Assignees
No one assigned
Labels
type: refactoring
Projects
None yet
Milestone
3.2.0-RC1
Development

Successfully merging this pull request may close these issues.
4 participants
@ngocnhan-tran1996 @artembilan @panchenko @acogoluegnes